Add mesh boundary gluing (post facial-adjacency overhaul) by majosm · Pull Request #253 · inducer/meshmode

majosm · 2021-08-09T21:50:55Z

~~Possibly~~ supersedes #204, ~~pending review/merge of the changes from #252.~~

majosm · 2021-09-24T20:28:43Z

I think this is ready for a look (apart from the one TODO I mentioned above).

inducer

Thanks! This looks super clean. I like it a lot. Here's some initial progress on reviewing this.

inducer · 2021-09-27T05:02:11Z

+    transformation (though the vertex ordering need not be the same).
+
+    :arg glued_boundary_mappings: a :class:`list` of tuples
+        *(btag_m, btag_n, aff_map, tol)* which each specify a mapping between two


I think four is big enough to warrant a dataclass. What do you think?

I ended up just grouping the first 3 arguments and leaving the tolerance separate. (4664575)

inducer · 2021-09-27T05:03:26Z

+    glued_boundary_mappings = []
+    for btag_m, btag_n, aff_map, tol in partial_glued_boundary_mappings:
+        glued_boundary_mappings.append((btag_m, btag_n, aff_map, tol))
+        if (btag_n, btag_m) not in partial_btag_pairs:


Should this case be banned?

I actually went the other direction and banned passing both. I like the convenience of just passing one mapping; the code can figure out the inverse.

inducer · 2021-09-27T05:08:03Z

+# FIXME: This tree-based approach is probably slow; see if there's a way to do
+# something like this using numpy constructs


We could check how slow the O(N^2) think with numpy is and where the cross-over point with the tree is. A somewhat numpy-ish process to do this is here:

meshmode/meshmode/discretization/connection/direct.py

Lines 296 to 309 in d136649

dist_vecs = (ibatch.result_unit_nodes.reshape(dim, -1, 1)

- from_grp.unit_nodes.reshape(dim, 1, -1))

dists = la.norm(dist_vecs, axis=0, ord=2)

result = np.zeros(ntgt_nodes, dtype=self.to_discr.mesh.element_id_dtype)

for irow in range(ntgt_nodes):

close_indices, = np.where(dists[irow] < tol)

if len(close_indices) != 1:

return None

close_index, = close_indices

result[irow] = close_index

Note that there's a Python loop left still.

I added the numpy version. (06565f4) On my laptop the numpy version is faster when the number of boundary vertices is less than 10k or so.

Co-authored-by: Andreas Klöckner <inform@tiker.net>

…post-adj-overhaul

inducer · 2021-10-04T20:27:11Z

+        is the tolerance allowed between the vertex coordinates of *btag_n* and the
+        transformed vertex coordinates of *btag_m* when attempting to match the two.
+    """
+    glued_boundary_mappings = _complete_glued_boundary_mappings(


The way it's currently structured, I think this calls _match_boundary_faces twice for each input "mapping pair". (Once for a->b and once for b->a.) Eliminate or add a comment to justify.

Fixed. (2ddc9ec)

inducer · 2021-10-04T20:37:14Z

+                h = 1/mesh_par
+            else:
+                mesh = mgen.generate_annular_cylinder_slice_mesh(
+                    mesh_par, (1, 2, 3), 0.5, 1, periodic=True)


f is not periodic on that mesh though?

Turns out it is, just because the mesh is symmetric about the x-axis and so is f.

inducer · 2021-10-04T22:28:59Z

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

…post-adj-overhaul

…lar_rect_mesh

not needed anymore, I think

inducer

LGTM, thanks! I'll apply the one suggestion.

To avoid giant temporaries.

inducer · 2021-10-20T20:11:16Z

@majosm Would you like to manually rebase or are you OK with me squashing this?

majosm · 2021-10-20T20:35:48Z

@majosm Would you like to manually rebase or are you OK with me squashing this?

Looks OK to squash I think.

inducer · 2021-10-20T20:45:42Z

Sounds good. In it goes! Thanks again for your work on this.

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch 2 times, most recently from d060ad9 to 0511de1 Compare August 9, 2021 22:17

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch from 0511de1 to f556ec5 Compare September 21, 2021 21:49

majosm added 6 commits September 23, 2021 09:23

add mesh boundary gluing

bd207ab

add generate_annular_cylinder_slice_mesh

2af9d39

add tests for facial adjacency transforms and mesh boundary gluing

8d645a3

check for facial adjacency transforms in map_mesh

d51a691

add facial adjacency transform handling in affine_map

607755a

add periodic argument to generate_box_mesh/generate_regular_rect_mesh

b18b5a7

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch from f556ec5 to 6ae6b0e Compare September 23, 2021 14:24

majosm mentioned this pull request Sep 23, 2021

Add mesh boundary gluing (aka periodic) #204

Closed

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch from 6ae6b0e to b18b5a7 Compare September 23, 2021 14:27

majosm commented Sep 23, 2021

View reviewed changes

Comment thread meshmode/mesh/processing.py Outdated

majosm marked this pull request as ready for review September 24, 2021 20:27

majosm requested a review from inducer September 24, 2021 20:27

inducer reviewed Sep 27, 2021

View reviewed changes

majosm and others added 2 commits October 4, 2021 13:37

make arguments keyword-only in generate_box_mesh

d772f4f

Co-authored-by: Andreas Klöckner <inform@tiker.net>

Merge remote-tracking branch 'origin/main' into mesh-boundary-gluing-…

805933a

…post-adj-overhaul

inducer reviewed Oct 4, 2021

View reviewed changes

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch from 02a995c to 7252fee Compare October 4, 2021 21:54

majosm added 6 commits October 7, 2021 16:33

Merge remote-tracking branch 'origin/main' into mesh-boundary-gluing-…

a63c091

…post-adj-overhaul

reference glue_mesh_boundaries in generate_box_mesh and generate_regu…

b012ba3

…lar_rect_mesh

fix doc reference

8448c94

report information about vertices that could not be matched

8d08353

disable nodal adjacency in glued meshes

4f799ea

use list/set comprehensions

6484478

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch from 7252fee to a75aa7c Compare October 7, 2021 21:34

majosm added 7 commits October 8, 2021 15:46

use dataclass for boundary mappings

4664575

split mapping_face_id_pairs into two variables

bb0a2ea

remove sorting matching faces by group

b640be2

not needed anymore, I think

specify order of intra-match indices in _match_faces_by_vertices

70bfaf0

document _match_boundary_faces

d4e9ab4

don't do face matching twice

2ddc9ec

add numpy vertex matching for small cases

06565f4

majosm force-pushed the mesh-boundary-gluing-post-adj-overhaul branch from a75aa7c to 06565f4 Compare October 8, 2021 20:47

majosm requested a review from inducer October 8, 2021 21:39

Merge branch 'main' into mesh-boundary-gluing-post-adj-overhaul

97907e2

inducer approved these changes Oct 20, 2021

View reviewed changes

Comment thread meshmode/mesh/processing.py Outdated

Bump down cutoff for tree algorithm in face matching

e28f594

To avoid giant temporaries.

inducer enabled auto-merge (rebase) October 20, 2021 17:27

auto-merge was automatically disabled October 20, 2021 18:20
Rebase failed

inducer merged commit 9898f3c into inducer:main Oct 20, 2021

		# FIXME: This tree-based approach is probably slow; see if there's a way to do
		# something like this using numpy constructs

	dist_vecs = (ibatch.result_unit_nodes.reshape(dim, -1, 1)
	- from_grp.unit_nodes.reshape(dim, 1, -1))
	dists = la.norm(dist_vecs, axis=0, ord=2)

	result = np.zeros(ntgt_nodes, dtype=self.to_discr.mesh.element_id_dtype)

	for irow in range(ntgt_nodes):
	close_indices, = np.where(dists[irow] < tol)

	if len(close_indices) != 1:
	return None

	close_index, = close_indices
	result[irow] = close_index

Conversation

majosm commented Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

majosm commented Sep 24, 2021

Uh oh!

inducer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inducer commented Oct 4, 2021

Uh oh!

inducer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inducer commented Oct 20, 2021

Uh oh!

majosm commented Oct 20, 2021

Uh oh!

inducer commented Oct 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

majosm commented Aug 9, 2021 •

edited

Loading